Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v6.x backport] build: use generic names for linting tasks #16297

Closed
wants to merge 35 commits into from
Closed

[v6.x backport] build: use generic names for linting tasks #16297

wants to merge 35 commits into from

Conversation

seishun
Copy link
Contributor

@seishun seishun commented Oct 18, 2017

Backport of #15272. Not sure if anyone is going to benefit from it but 🤷‍♂️ .

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

build

danbev and others added 30 commits October 17, 2017 16:05
Currently the variables set in vcbuild.bat are mostly global and
escape/leak out into the calling process. For example, running
vcbuild.bat test and then echoing the config variable gives:

vcbuild.bat test
...
echo %config%
Release

After this change the same command give:
vcbuild.bat test
...
echo %config%
%config%

PR-URL: #15754
Reviewed-By: James M Snell <jasnell@gmail.com>
In code example `vm.createContext` called with new operator by mistake.
It is not a constructor.

PR-URL: #16059
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
While VM module's columnOffset option does succeed in applying an offset
to the column number in the stack trace, the wavy diagram printed does
not account for potential offsets, resulting in erroneous location of
`^` in the first line of the script.

Before:

```
> vm.runInThisContext('throw new Error()', { columnOffset: 5 })
evalmachine.<anonymous>:1
throw new Error()
     ^

Error
    at evalmachine.<anonymous>:1:12
    at ContextifyScript.Script.runInThisContext (vm.js:44:33)
    at Object.runInThisContext (vm.js:116:38)
```

After:

```
> vm.runInThisContext('throw new Error()', { columnOffset: 5 })
evalmachine.<anonymous>:1
throw new Error()
^

Error
    at evalmachine.<anonymous>:1:12
    at ContextifyScript.Script.runInThisContext (vm.js:50:33)
    at Object.runInThisContext (vm.js:139:38)
    at repl:1:4
```

PR-URL: #15771
Refs: jsdom/jsdom#2003
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
PR-URL: #16014
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
There are no longer files in the repository that use the `.markdown`
extension so remove mention of them.

PR-URL: #15786
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Lance Ball <lball@redhat.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
PR-URL: #15978
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
PR-URL: #15974
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #15972
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Updated console example to follow style of rest of the examples

PR-URL: #15962
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
PR-URL: #15957
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #15956
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: #15954
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
PR-URL: #15937
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Assertions will now print the values that caused the assertions
to fail.

PR-URL: #15928
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Added interpolated strings to display the error value

PR-URL: #15926
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
This commit makes understanding assertion failures easier by
displaying the values that failed the assertion.

PR-URL: #15883
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
PR-URL: #15911
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Adding back the changes made by commit# 791d560, that suggests running
macosx-firewall.sh script after bulid step. These changes were deleted
by commit# fc102d0, but they are still applicable.

PR-URL: #15829
Reviewed-By: Ryan Graham <r.m.graham@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
PR-URL: #15921
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #15920
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Add result to part of assert message.

PR-URL: #15918
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #15915
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #15914
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #15910
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #15944
Reviewed-By: Lance Ball <lball@redhat.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #15906
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
remove third argument `0` from calls to `assert.deepStrictEqual`

PR-URL: #15896
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #16039
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #16046
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Lance Ball <lball@redhat.com>
PR-URL: #16079
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
kfarnung and others added 5 commits October 17, 2017 16:45
PR-URL: #16108
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Bryan English <bryan@bryanenglish.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
PR-URL: #16019
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Lance Ball <lball@redhat.com>
PR-URL: #15997
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Replace string concatenation in `tools/lint-js.js` with template
literals.

PR-URL: #15858
Reviewed-By: Rich Trott <rtrott@gmail.com>
"jslint" is the name of a tool that actually is not used, which can
cause confusion.

PR-URL: #15272
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. tools Issues and PRs related to the tools directory. v6.x windows Issues and PRs related to the Windows platform. labels Oct 18, 2017
@seishun
Copy link
Contributor Author

seishun commented Oct 18, 2017

Copy link
Member

@gibfahn gibfahn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll benefit from consistent make targets across releases.

MylesBorins pushed a commit that referenced this pull request Oct 19, 2017
"jslint" is the name of a tool that actually is not used, which can
cause confusion.

Backport-PR-URL: #16297
PR-URL: #15272
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
@MylesBorins
Copy link
Contributor

landed in 72f2646
Should we consider backporting to v4.x as well?

@seishun
Copy link
Contributor Author

seishun commented Oct 19, 2017

Probably too much effort for its worth.

MylesBorins pushed a commit that referenced this pull request Oct 25, 2017
"jslint" is the name of a tool that actually is not used, which can
cause confusion.

Backport-PR-URL: #16297
PR-URL: #15272
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
MylesBorins pushed a commit that referenced this pull request Oct 26, 2017
"jslint" is the name of a tool that actually is not used, which can
cause confusion.

Backport-PR-URL: #16297
PR-URL: #15272
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. tools Issues and PRs related to the tools directory. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

Successfully merging this pull request may close these issues.